Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[TOPI] Alleviate hanging issue caused by concat op #3268

Closed
wants to merge 2 commits into from

Conversation

Laurawly
Copy link
Contributor

@tqchen This is a temporary solution to alleviate the problem, will work on the IR builder to write the schedule for concate op.

@tqchen
Copy link
Member

tqchen commented May 31, 2019

cc @jroesch @wweic the CI problem could relate to limitations in VM

@masahi
Copy link
Member

masahi commented May 31, 2019

operator fusion tests are heavily dependent on concat being fusible as injective. If we want to make concat opaque, corresponding updates to fusion tests are needed.

If this is indeed a temporary solution, I suggest just disable fusion tests that involve concat.

@jroesch
Copy link
Member

jroesch commented Jun 1, 2019

I have a patch for the VM issue which is almost complete, will post tomorrow sometime.

@kevinthesun
Copy link
Contributor

Will this change affect the performance of concat for cpu?

@Laurawly
Copy link
Contributor Author

Laurawly commented Jun 2, 2019

Will this change affect the performance of concat for cpu?

Previously ssd_mobilenet1.0_512 cost 1.4437s on arm CPU now it costs 1.4545s.

@kevinthesun
Copy link
Contributor

I tested on x86 cpu with a set of gluoncv models. This change doesn't affect the performance.

@apivovarov
Copy link
Contributor

apivovarov commented Jun 6, 2019

This fix solves 10-15 min hanging issue for the first inference
But it causes 20% performance degradation for the second and consequent inferences on GPU.
I tested it for GluonCV ssd_512_mobilenet1.0_voc model on Mali GPU OpenCL (RK3399)
The second and consequent inferences take:
before the fix: 420ms
after the fix: 520ms

@Laurawly
Copy link
Contributor Author

Laurawly commented Jun 6, 2019

This fix solves 10-15 min hanging issue for the first inference
But it causes 20% performance degradation for the second and consequent inferences on GPU.
I tested it for GluonCV ssd_512_mobilenet1.0_voc model on Mali GPU OpenCL (RK3399)
The second and consequent inferences take:
before the fix: 420ms
after the fix: 520ms

The performance reported by tvm evaluator with hanging issue was 470 ms on Mali GPU RK3399. But the hanging time is more than 10 minutes which composes most of the end-to-end time. And by this fix, the hanging issue is alleviated, and performance evaluated by tvm time evaluator is 517 ms. You are more than welcome to fix the hanging issue without performance lost.

@Laurawly
Copy link
Contributor Author

Laurawly commented Jun 6, 2019

operator fusion tests are heavily dependent on concat being fusible as injective. If we want to make concat opaque, corresponding updates to fusion tests are needed.

If this is indeed a temporary solution, I suggest just disable fusion tests that involve concat.

@tqchen Shall we disable fusion tests that involve concat?

@hlu1
Copy link
Contributor

hlu1 commented Jun 7, 2019

@apivovarov, making Concat opaque allows you to write fast schedules specially for concat, for instance, https://github.com/dmlc/tvm/blob/master/topi/python/topi/x86/injective.py#L53 for x86 concat schedules. Note that the concat schedule is only guaranteed to be used only when concat is an opaque or output_elem_fusable op. The default injective schedule would be used when concat is an injective op and fused together with other injective ops.

@tqchen
Copy link
Member

tqchen commented Jun 7, 2019

Some related followup thoughts https://discuss.tvm.ai/t/explore-optimizations-for-concat/2435/7

@icemelon icemelon added the status: need update need update based on feedbacks label Jun 18, 2019
@tqchen
Copy link
Member

tqchen commented Sep 13, 2019

Close this for now due to inactive status, @sxjscience has volunteered to continue working on the thread https://discuss.tvm.ai/t/explore-optimizations-for-concat/2435/7

@tqchen tqchen closed this Sep 13, 2019
@tqchen tqchen added status: inactive and removed status: need update need update based on feedbacks labels Sep 13, 2019
@sxjscience
Copy link
Member

@tqchen I'm still working on that. Need to wait for some more time...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

9 participants